test(review): document the last unreachable review-evasion coverage branch - #3476
Conversation
…ranch closePullRequest's only failure path is Octokit's request() call, which always rejects with a RequestError (an Error subclass) -- the `: new Error(...)` normalization branch is a type-safety fallback for the unknown-typed catch, not a reachable runtime path. Closes out patch coverage on the review-evasion protection PR: 100% line/branch across all 10 changed files.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-05 09:53:28 UTC
⏸️ Suggested Action - Manual Review
Review summary Blockers
Nits — 3 non-blocking
Concerns raised — review before merging
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3476 +/- ##
=======================================
Coverage 93.03% 93.03%
=======================================
Files 298 298
Lines 31203 31203
Branches 11386 11386
=======================================
+ Hits 29029 29030 +1
Misses 1518 1518
+ Partials 656 655 -1
🚀 New features to boost your workflow:
|
…ranch Adds a v8-ignore annotation for the defensive non-Error normalization arm in closeReviewEvasionSelfCloseIfActive's re-close-failure path. closePullRequest's only failure mode is Octokit's request() call, which always rejects with a RequestError (an Error subclass), never a raw thrown value -- the normalization exists only to satisfy the unknown-typed catch's type signature. Split the ternary into an if block with the ignore scoped to only the unreachable else/fallback, so the reachable Error rethrow path stays fully subject to coverage. No functional change.
…hrow Names the specific existing regression test that exercises the `closeError instanceof Error` rethrow in closeReviewEvasionSelfCloseIfActive, so the branch-coverage annotation's claim is directly verifiable rather than asserted. No functional change.
…view freeze (#3490) The maintainer-gated freeze (#3461) reused the last published AI review for any PR carrying the manual-review label, with no author exemption. Confirmed live on PR #3476: pushing genuine follow-up commits to the owner's own held PR kept replaying the ORIGINAL, now-stale AI verdict instead of evaluating the new commits, because github_app.ai_review_frozen_reuse fired on every push. The anti-gaming concern the freeze exists for is specific to a contributor iterating pushes against the bot; it never applies to the repo owner, an ADMIN_GITHUB_LOGINS fleet operator, or a protected automation bot, matching the exemption this codebase already grants those authors everywhere else (auto-close, review-nag, contributor caps). Root-caused via the live audit_events trail on the self-host VPS, which showed ai_review_frozen_reuse firing on every one of #3476's own follow-up commits despite the PR being owner-authored.
Summary
/* v8 ignore next */annotation with an explanatory comment on the one remaining defensive fallback branch incloseReviewEvasionSelfCloseIfActive:closeError instanceof Error ? closeError : new Error(errorMessage(closeError)).closePullRequest's only failure path is Octokit'srequest()call, which always rejects with aRequestError(anErrorsubclass) -- thenew Error(...)normalization arm is a type-safety fallback for theunknown-typed catch, not a reachable runtime path. Documents that explicitly rather than leaving it silently uncovered.closeError instanceof Errorrethrow path too. Rewrote it as anifblock (if (closeError instanceof Error) throw closeError;) with/* v8 ignore else */scoping the ignore to only the unreachable non-Error fallback, so the reachable rethrow stays fully subject to coverage.No issue because this is a maintenance-only coverage annotation (chore): a one-line
v8 ignorecomment tightening patch coverage on an already-merged PR (#3414), with no functional or behavioral change to review against.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlint(n/a -- no workflow changes)npm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workers(n/a -- no workers-pool test changes)npm run build:mcp(n/a -- no MCP package changes)npm run test:mcp-pack(n/a -- no MCP package changes)npm run ui:openapi:check(n/a -- no OpenAPI/schema changes)npm run ui:lint(n/a -- no UI changes)npm run ui:typecheck(n/a -- no UI changes)npm run ui:build(n/a -- no UI changes)npm audit --audit-level=moderatetest/unit/queue.test.tsre-run clean)If any required check was skipped, explain why:
src/queue/processors.ts); the workers/mcp/ui suites are unaffected by a comment change, so onlytypecheckand the affected test file (test/unit/queue.test.ts, review-evasion suite) were re-run directly, per this repo's usual scoped-validation practice for small follow-ups.Safety
UI Evidencesection below with screenshots. (n/a -- no visible UI)